-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BEP-341: Validators can produce consecutive blocks #2482
BEP-341: Validators can produce consecutive blocks #2482
Conversation
3193cf0
to
6f8423c
Compare
417a19f
to
bb16185
Compare
e705707
to
e1723e1
Compare
356d364
to
f513d20
Compare
9f663bf
to
6589d44
Compare
a141a9b
to
bd82176
Compare
} | ||
snap.Recents[number] = validator | ||
snap.RecentForkHashes[number] = hex.EncodeToString(header.Extra[extraVanity-nextForkHashSize : extraVanity]) | ||
snap.updateAttestation(header, chainConfig, s.config) | ||
// change validator set | ||
if number > 0 && number%s.config.Epoch == uint64(len(snap.Validators)/2) { | ||
if number > 0 && number%s.config.Epoch == snap.minerHistoryCheckLen() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not snap.minerHistoryCheckLen() + 1
to change validator set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not
snap.minerHistoryCheckLen() + 1
to change validator set?
when turnLenght==1, snap.minerHistoryCheckLen() == uint64(len(snap.Validators)/2), exactly the same as before.
why not +1
here?
for example when turnLenght ==1,
[200, 210] contains 11 validators, is a majority of validators, so can do the change at 210, not wait to 211.
snap.Validators = newVals | ||
if chainConfig.IsLuban(header.Number) { | ||
validators := snap.validators() | ||
for idx, val := range validators { | ||
snap.Validators[val].Index = idx + 1 // offset by 1 | ||
} | ||
} | ||
for i := snap.versionHistoryCheckLen(); i < oldVersionsLen; i++ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this apply(...)
is not code review friendly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can compare to the old logic about deleting RecentForkHashes
,
the effect is the same as before, but the code is much more simplified
} | ||
|
||
// inturnValidator returns the validator at a given block height. | ||
func (s *Snapshot) inturnValidator() common.Address { | ||
validators := s.validators() | ||
offset := (s.Number + 1) % uint64(len(validators)) | ||
offset := (s.Number + 1) / uint64(s.TurnLength) % uint64(len(validators)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(s.Number + s.TurnLength) / uint64(s.TurnLength)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(s.Number + s.TurnLength) / uint64(s.TurnLength)
?
when at height s.Number + 1
, we can get snapshot with s.Number.
the function name is not quite clear. I will update the comments for the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(s.Number + s.TurnLength) == (s.Number + 1 + (s.TurnLength -1))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(s.Number + s.TurnLength) == (s.Number + 1 + (s.TurnLength -1))
not enable the first validator after switch go generate s.TurnLength blocks
Description
BEP-341: Validators can produce consecutive blocks
related change in system contracts
Rationale
tell us why we need these changes...
Example
add an example CLI or API response...
Changes
Notable changes: